Skip to content

add test solution#800

Open
arturgorniak wants to merge 25 commits intomate-academy:masterfrom
arturgorniak:develop
Open

add test solution#800
arturgorniak wants to merge 25 commits intomate-academy:masterfrom
arturgorniak:develop

Conversation

@arturgorniak
Copy link
Copy Markdown

@mkyloa mkyloa closed this Jun 6, 2023
@mkyloa mkyloa reopened this Jun 6, 2023
@mkyloa mkyloa requested a review from ihor-jpeg June 6, 2023 10:51
@mkyloa mkyloa closed this Jun 6, 2023
@mkyloa mkyloa reopened this Jun 6, 2023
Copy link
Copy Markdown

@mkyloa mkyloa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

Copy link
Copy Markdown

@mkyloa mkyloa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

@mkyloa mkyloa self-requested a review June 6, 2023 11:00
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix HTML formating and remove unnecessary class names

Comment thread src/index.html Outdated
Comment on lines +6 to +7
<meta name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix formatting

Suggested change
<meta name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"
<meta
name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"
>

Comment thread src/index.html Outdated

<body>
<h1>HTML Form</h1>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces between parent and child are not needed, remove

Comment thread src/index.html Outdated
Comment on lines +16 to +17
<form action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post" name="personal-information">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fiz formatting:
Also name should be camelCase

Suggested change
<form action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post" name="personal-information">
<form
action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post"
name="personal-information"
>

Comment thread src/index.html Outdated
Comment on lines +22 to +29
<label for="surname">Surname:
<input
type="text"
autocomplete="off"
id="surname"
name="surname"
>
</label>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix formatting:

Suggested change
<label for="surname">Surname:
<input
type="text"
autocomplete="off"
id="surname"
name="surname"
>
</label>
<label for="surname">
Surname:
<input
type="text"
autocomplete="off"
id="surname"
name="surname"
>
</label>

Comment thread src/index.html Outdated
</div>

<div class="input name">
<label for="name">Name:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, check all documnent and fix formatting

Comment thread src/index.html Outdated
>
</label>
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove line

Comment thread src/index.html Outdated
<input
type="radio"
name="cats"
id="cats yes"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not add two ids, you can use for example catLoverYes

Comment thread src/index.html

<div class="input cars">
<label for="cars">What are your favorite brand of cars?
<select
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix formatting

Comment thread src/index.html Outdated
<form action="https://mate-academy-form-lesson.herokuapp.com/create-application"
method="post" name="personal-information">

<fieldset class="form-field personal-information">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You dont need additional classes for fieldset, remove in whole HTML and use only form-field class, I would suggest to rename it to form-fieldset for consistancy

Comment thread src/index.html Outdated

<fieldset class="form-field personal-information">
<legend>Personal information</legend>
<div class="input surname">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, use single class for styling input container and name class more descriptively like form-field

@arturgorniak
Copy link
Copy Markdown
Author

I made changes as suggested, it should be fine now.

Copy link
Copy Markdown

@darokrk darokrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix code indentations in whole file and we are ready to go 👍🏽

Comment thread src/index.html

<body>
<h1>HTML Form</h1>
<form
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the formatting of the code. You have to use the correct code indentations.

Look Dorota's mentions before and apply it in the whole code :)

    <form 
       action="https://mate-academy-form-lesson.herokuapp.com/create-application"
       method="post"
       name="personalInformation"
    >

@arturgorniak
Copy link
Copy Markdown
Author

done

@arturgorniak arturgorniak requested a review from darokrk June 9, 2023 14:40
Copy link
Copy Markdown

@DorotaLeniecDev DorotaLeniecDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more and we are good to go

Comment thread src/index.html Outdated
Comment on lines +5 to +7
<meta
charset="UTF-8"
>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall when there is less than 3 attributes in a tag there is no need for spreading it into several lines. We do it when the attribute list is long for readability so this should simply be:

Suggested change
<meta
charset="UTF-8"
>
<meta charset="UTF-8">

Comment thread src/index.html Outdated
Comment on lines +10 to +11
content="width=device-width, user-scalable=no, initial-scale=1.0,
maximum-scale=1.0, minimum-scale=1.0"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont spread attribute value, put it in a one line

Suggested change
content="width=device-width, user-scalable=no, initial-scale=1.0,
maximum-scale=1.0, minimum-scale=1.0"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"

Comment thread src/index.html
Comment on lines +132 to +144
<input
type="radio"
name="cats"
id="catsYes"
>
Yes

<input
type="radio"
name="cats"
id="catsNo"
>
No
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make Yes and No values a clicable lables so it focuses on the correct input when clicked

Comment thread src/index.html
<label for="cars">
What are your favorite brand of cars?

<select
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix formating for this select tag, add indentations

Comment thread src/index.html Outdated
Comment on lines +177 to +179
name="cars"
id="cars"
multiple>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix formating for this select tag, add indentations like:

Suggested change
name="cars"
id="cars"
multiple>
<select
name="cars"
id="cars"
multiple
>

Comment thread src/index.html Outdated
type="submit"
value="Submit"
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line

@arturgorniak
Copy link
Copy Markdown
Author

Finally i had some time to fix this. Can i also please to run tests for hello world task?
https://github.com/arturgorniak/layout_hello-world

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants